Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce Passthrough Response type #300

Merged
merged 14 commits into from
Jun 2, 2020

Conversation

jyotimahapatra
Copy link
Contributor

@jyotimahapatra jyotimahapatra commented May 15, 2020

#299

While implementing envoyproxy/xds-relay#85 , we realized that go-control-plane always marshals the response. As a result, we were not able to send an existing DiscoveryResponse as a passthrough.
This PR introduces a new PassthroughResponse struct , which is not subjected to any marshalling while sending the response on the watches.

Jyoti Mahapatra added 6 commits May 15, 2020 12:50
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
@jyotimahapatra
Copy link
Contributor Author

@jessicayuen This is a POC for conveying my thoughts on #299 . Let me know what you think

pkg/cache/v2/cache.go Outdated Show resolved Hide resolved
pkg/cache/v2/cache.go Outdated Show resolved Hide resolved
pkg/cache/v2/cache.go Outdated Show resolved Hide resolved
Jyoti Mahapatra added 2 commits May 15, 2020 14:32
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
pkg/cache/v2/cache.go Outdated Show resolved Hide resolved
Jyoti Mahapatra added 3 commits May 15, 2020 15:10
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
@jyotimahapatra jyotimahapatra changed the title [Experiment]marshal Introduce Passthrough Response type May 15, 2020
@jyotimahapatra
Copy link
Contributor Author

@jessicayuen PTAL. Once it looks good to you, we can ask Kuat to take a look next.

Copy link
Member

@jessicayuen jessicayuen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, just a few minor comments:

pkg/cache/v2/cache.go Show resolved Hide resolved
pkg/cache/v2/cache.go Outdated Show resolved Hide resolved
pkg/cache/v2/cache.go Show resolved Hide resolved
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
@jyotimahapatra
Copy link
Contributor Author

@kyessenov Please take a look.

Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
@jyotimahapatra
Copy link
Contributor Author

@kyessenov Let us know what you feel about the change.

@mattklein123 mattklein123 self-assigned this May 26, 2020
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay. Makes sense to me. (I didn't review the tests.) Just a small typo I saw.

pkg/cache/v2/cache.go Outdated Show resolved Hide resolved
Signed-off-by: Jyoti Mahapatra <jmahapatra@lyft.com>
@mattklein123 mattklein123 merged commit b12a9ed into envoyproxy:master Jun 2, 2020
@jyotimahapatra jyotimahapatra deleted the marshal branch June 2, 2020 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants